fix: retain stream read error on subsequent response.content accesses (#4965)#7527
Closed
armorbreak001 wants to merge 1 commit into
Closed
fix: retain stream read error on subsequent response.content accesses (#4965)#7527armorbreak001 wants to merge 1 commit into
armorbreak001 wants to merge 1 commit into
Conversation
…psf#4965) When accessing response.content raises an exception during stream reading (e.g., ConnectionError, ChunkedEncodingError), subsequent accesses would return empty bytes (b"") or raise a generic RuntimeError instead of re-raising the original error. This made debugging especially difficult in debuggers where properties may be accessed multiple times, as the original error was silently lost. Changes: - Add _content_error attribute to cache the first read exception - Check _content_error early in content property (before re-read attempt) - Re-raise the same exception object on subsequent accesses - Add regression tests for full-error and partial-read failure cases Fixes psf#4965
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #4965 (supersedes #7415)
When
response.contentraises an exception during stream reading (e.g.,ConnectionError,ChunkedEncodingError), subsequent accesses would silently return an empty bytestring or raise a genericRuntimeErrorinstead of re-raising the original error.This is especially problematic in debuggers where properties may be accessed multiple times — the original error is silently lost, making debugging very difficult.
Root Cause
In the
contentproperty:if self._content is False, callsiter_content()iter_content()raises (e.g., connection reset mid-stream), the exception propagates_content_consumedwas already set to True insideiter_content()_content is Falseis still True, but now hits the_content_consumedguard → raises genericRuntimeError_contentmay be set to incomplete dataChanges
_content_errorattribute to cache the first read exception_content_errorbefore the_content is Falseblock (addresses reviewer feedback on Fix: retain stream read error on subsequent response.content access #7415)Testing
Two new regression tests in
test_lowlevel.py:test_response_content_retains_error: Full read failure → second access raises sameConnectionErrortest_response_content_retains_error_after_partial_read: Partial read then failure → second access raises sameChunkedEncodingErrorBoth tests verify that the exception type and identity are preserved across multiple accesses.
Difference from #7415
_content_errorin__init__(not dynamically in property)_content_errorbefore theif _content is Falseguard (prevents unnecessary re-read attempt)